strings: add Builder.write_u_decimal and write_decimal JS-backend parity - #27522
Conversation
Builder.write_decimal(i64) was added in vlang#19625, but there was no unsigned variant (values above max_i64 had to go through n.str(), which allocates), and the JS backend Builder had no write_decimal at all, so code using it was not portable across backends. - add write_u_decimal(n u64) to builder.c.v (zero-alloc, full u64 range) - mirror write_decimal and write_u_decimal into builder.js.v - add test_write_u_decimal covering 0, max_i64, max_i64 + 1 and max_u64 Fixes vlang#27510 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 811733e9d8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| b.write_u8(0x30) | ||
| return | ||
| } | ||
| if n == min_i64 { |
There was a problem hiding this comment.
On the JS backend a true -9223372036854775808 value can still be produced at runtime, for example with '-9223372036854775808'.i64(), but the backend's min_i64 constant is known to lower incorrectly there, so this guard is skipped. The following digit loop then negates/formats the signed 64-bit minimum through the normal path, which is exactly the overflow case the C implementation special-cases, so write_decimal is not actually parity-correct for that input.
Useful? React with 👍 / 👎.
…4 constant Address the review on vlang#27522: on the JS backend the min_i64 constant lowers incorrectly, so the 'if n == min_i64' guard was skipped and a genuine runtime min_i64 (e.g. from '-9223372036854775808'.i64()) fell into the digit loop where negating it overflows i64. Compute the magnitude with wrapping unsigned arithmetic (u64(0) - u64(n)) and delegate to write_u_decimal instead. This is parity-correct for runtime min_i64 on both backends, removes the dependence on the constant, and drops the allocating n.str() fallback so the C path is now allocation-free for every input.
3 participants
Builder.write_decimal(i64)(the zero-alloc decimal writer added in #19625) had twoparity gaps:
u64variant. The API isi64-typed, so values abovemax_i64could not be written allocation-free — callers had to fall back to
write_string(n.str()), which allocates.write_decimalat all. It lived only inbuilder.c.v, socode relying on it did not compile on the JS backend.
This PR:
pub fn (mut b Builder) write_u_decimal(n u64)tobuilder.c.v— same stack-bufferapproach as
write_decimal, no sign branch, covering the entireu64range;write_decimalandwrite_u_decimalintobuilder.js.v(usingwrite_u8,since the JS backend has no
write_ptr);test_write_u_decimalcovering0,max_i64,max_i64 + 1, andmax_u64.Tests
v test vlib/strings/builder_test.v→ OKv -silent test vlib/strings/→ 9 passed, 9 totalv -b js run):write_u_decimal(max_u64)→18446744073709551615, and normali64/u64values match the C backend.Note: on the JS backend the
min_i64constant already evaluates incorrectly (plainprintln(min_i64)/min_i64.str()print-1) — a pre-existing JS-backend i64limitation, unrelated to this change. All other values format correctly on both backends.
Fixes #27510